feat: Add presigned URL target models and cleanups#3486
Conversation
- add plus presigned artifact/result response models - introduce and helpers to separate internal results from wire models - add regular-only source request models, including ZIP URL rejection and S3 exclusion - export new datamodel types and add focused schema/serialization tests Signed-off-by: Christoph Auer <cau@zurich.ibm.com>
|
❌ DCO Check Failed Hi @cau-git, your pull request has failed the Developer Certificate of Origin (DCO) check. This repository supports remediation commits, so you can fix this without rewriting history — but you must follow the required message format. 🛠 Quick Fix: Add a remediation commitRun this command: git commit --allow-empty -s -m "DCO Remediation Commit for Christoph Auer <cau@zurich.ibm.com>
I, Christoph Auer <cau@zurich.ibm.com>, hereby add my Signed-off-by to this commit: cd1203f91fcd1f940337ece8f4fb9f0595f46abb"
git push🔧 Advanced: Sign off each commit directlyFor the latest commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits: git rebase --signoff origin/main
git push --force-with-leaseMore info: DCO check report |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Signed-off-by: Christoph Auer <cau@zurich.ibm.com>
| ) | ||
|
|
||
|
|
||
| class PresignedUrlConvertDocumentResponse(BaseModel): |
There was a problem hiding this comment.
Currently used for S3Target and PutTarget response. How can this be deprecated? PresignedUrlConvertResponse is not going to replace it, since we said that an S3 target cannot list documents produced by default as it may be very very large, need pagination etc.
| ) | ||
|
|
||
|
|
||
| def _to_convert_document_response( |
There was a problem hiding this comment.
Same question as for ExportResult, why is ConvertDocumentResponse and DocumentResultItem both needed? Appears to have 100% overlap.
If any of this is for backward-compatibility, please explain.
| def _to_export_result(item: DocumentResultItem) -> ExportResult: | ||
| return ExportResult( | ||
| content=item.document, | ||
| status=item.status, | ||
| errors=item.errors, | ||
| timings=item.timings, | ||
| ) |
There was a problem hiding this comment.
Why is ExportResult needed when we have DocumentResultItem, or vice versa?
The difference seems to be only:
- "content" instead of "document" field, same type.
- kind present on "ExportResult".
It makes no sense to me. If any of this is for backward-compatibility, please explain.
…semantics Signed-off-by: Christoph Auer <cau@zurich.ibm.com>
Signed-off-by: Christoph Auer <cau@zurich.ibm.com>
This reverts commit a622eed.
1 participant
This PR updates the shared
doclingservice datamodels to support upcoming server-managed artifact delivery, cleanly separate internal result construction from existing API response shapes, and tighten the regular convert endpoint request contract while preserving flexibility for future endpoint expansion.PresignedUrlTargetand corresponding artifact/result response models so conversion outputs can be represented as per-document downloadable artifact referencesDocumentResultItemplus mapping helpers to separate internal document result handling from legacy wire models such asExportResultandConvertDocumentResponse, without changing existing response payloadsChecklist: